Skip to content

try to fix github.com/vuejs/vue/issues/6353 #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

JounQin
Copy link

@JounQin JounQin commented Sep 6, 2017

What kind of change does this PR introduce?
bugfix

Did you add tests for your changes?
No

If relevant, did you update the README?
No

Summary
add _renderStyles property on context, so that we could define styles property in vue-server-renderer

Does this PR introduce a breaking change?
No

add `_renderStyles` property on context, so that we could define `styles` property in `vue-server-renderer`
@JounQin JounQin mentioned this pull request Sep 6, 2017
13 tasks
@yyx990803
Copy link
Member

see vuejs/vue#6522 (comment)

@yyx990803 yyx990803 closed this Sep 13, 2017
@JounQin JounQin deleted the patch-1 branch September 13, 2017 04:35
@yyx990803
Copy link
Member

Sorry, your original fix idea was better... I introduced a regression in my fix. Should've merged yours instead :)

@JounQin
Copy link
Author

JounQin commented Sep 15, 2017

Would you like to explain what's wrong with your original fix?

@yyx990803
Copy link
Member

yyx990803 commented Sep 15, 2017

The original fix was checking the presence of context.styles too early. At that point, only the root app instance has been created and the renderer hasn't started rendering the entire component tree yet, so no lifecycle styles would have been injected at that point. Creating context.styles there actually prevents vue-style-loader from creating it later, causing all lifecycle styles to be lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants